Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Query Filter Builder for Cookbooks and Meal Plans #4346

Merged

Conversation

michael-genson
Copy link
Collaborator

@michael-genson michael-genson commented Oct 10, 2024

What type of PR is this?

(REQUIRED)

  • bug
  • feature

What this PR does / why we need it:

(REQUIRED)

This PR implements a query filter builder for cookbooks and meal plans. Instead of relying on stored database relationships (cookbooks to tags, meal plan rules to categories, etc.) we now rely on a simple query filter string to filter our recipes. Way back when we implemented the query filter we talked about getting something like this implemented.

So, for instance, instead of:

class Cookbook:
  ...
  tags: list[Tag] = [t1, t2, t3]
  categories: list[Category] = [c1, c2, c3]

We have:

class Cookbook:
  ...
  query_filter_string: str = "tags.id CONTAINS ALL [t1, t2, t3] AND recipe_category.id CONTAINS ALL [c1, c2, c3]"

This has a few key benefits:

  1. Much greater filter control/flexibility. You're no longer limited to "includes all tags", "includes all categories", etc. You can do pretty much anything you want
  2. New fields/relationships can be added for free, as long as they exist on/are related to the recipe table. Since this uses a string, not database relationships, we don't need migrations to add/remove support
  3. Significantly simpler code (everything is handled by the query filter parser, so there's no need to handle a bunch of filter options)

While query filters aren't new to Mealie, the major work in this PR was getting it user-friendly. That's where the new query filter builder comes in. Here's a sample cookbook:
image

And a sample meal plan rule:
image

For advanced use cases, you can add parenthesis to group logical statements together:
image


A few non-obvious features:

  • You can toggle using parenthesis with the "Show Advanced" checkbox. I would argue most users won't use parenthesis (or at least don't need to use them) so turning them off saves window space
  • Saving is prevented if the query filter is invalid. A query filter is invalid if parenthesis aren't balanced, or a value is missing (e.g. CATEGORIES IN []
    image
  • You can re-order fields by dragging (note the handle on the left)
  • Everything is translatable (AND/OR/CONTAINS etc. all have translated labels and internal values)

Cookbooks were previously limited to only the household's recipes, but since it's trivial to add a household filter now that restriction has been lifted:
image

Existing cookbooks and meal plan rules are easily represented in the new query filter, and the migration adding the query filter string column handles this for you (e.g. cookbooks):

cookbooks = session.query(CookBook).all()
for cookbook in cookbooks:
    parts = []
    if cookbook.categories:
        relop = "CONTAINS ALL" if cookbook.require_all_categories else "IN"
        vals = ",".join([f'"{cat.id}"' for cat in cookbook.categories])
        parts.append(f"recipe_category.id {relop} [{vals}]")
    if cookbook.tags:
        relop = "CONTAINS ALL" if cookbook.require_all_tags else "IN"
        vals = ",".join([f'"{tag.id}"' for tag in cookbook.tags])
        parts.append(f"tags.id {relop} [{vals}]")
    if cookbook.tools:
        relop = "CONTAINS ALL" if cookbook.require_all_tools else "IN"
        vals = ",".join([f'"{tool.id}"' for tool in cookbook.tools])
        parts.append(f"tools.id {relop} [{vals}]")

    cookbook.query_filter_string = " AND ".join(parts)

Which issue(s) this PR fixes:

(REQUIRED)

Closes #3945
Closes #3205
Fixes #2200 (barely related to this PR, but it got fixed!)

Special notes for your reviewer:

(fill-in or delete this section)

The way the backend and frontend talk to each other regarding the query filter uses a new JSON model for the backend query filter builder. We already did the work of parsing the user-inputted string into components (see QueryFilterBuilder on the backend), I just added an extra step to make that a JSON object that the frontend can leverage:

class QueryFilterJSONPart(MealieModel):
    left_parenthesis: str | None = None
    right_parenthesis: str | None = None
    logical_operator: LogicalOperator | None = None

    attribute_name: str | None = None
    relational_operator: RelationalKeyword | RelationalOperator | None = None
    value: str | list[str] | None = None


class QueryFilterJSON(MealieModel):
    parts: list[QueryFilterJSONPart] = []

Cookbooks and meal plan rules now return the parsed query filter string with the API, so the frontend can take that information and reconstruct the query filter builder. e.g.

class ReadCookBook(UpdateCookBook):
    query_filter: Annotated[QueryFilterJSON, Field(validate_default=True)] = None  # type: ignore

    model_config = ConfigDict(from_attributes=True)

    @field_validator("query_filter", mode="before")
    def validate_query_filter(cls, _, info: ValidationInfo) -> QueryFilterJSON:
        try:
            query_filter_string: str = info.data.get("query_filter_string") or ""
            builder = QueryFilterBuilder(query_filter_string)
            return builder.as_json_model()
        except Exception:
            logger.exception(f"Invalid query filter string: {query_filter_string}")
            return QueryFilterJSON()

Since that JSON is derived from the parsed query filter string it's not stored in the database anywhere (we just store the query filter string). Since you can input invalid strings via the API, we have some checks that fix or otherwise ignore bad filter strings. The query filter builder only outputs valid strings (unless there's a bug of course).

This PR is a breaking change because we changed the API for cookbooks and meal plan rules. However, for the average user, since we migrate existing data to use query filters it's a low-impact change.

In the future I would love to get this working for the front page (maybe as a separate option alongside other options) but there are a lot more considerations there.

Testing

(fill-in or delete this section)

Added/updated pytest for the new query filter string handling. Thankfully most of our tests were easily adaptable since previous functionality was strictly a subset of query filters.

For the frontend, I played around with the editor a lot. I wasn't able to find any way of breaking it (e.g. invalid query filter strings or invalid values). The organizers were broken originally (see #2200) but I fixed those. Specifically switching a field from one organizer to another wasn't working (e.g. create a field for categories, enter a bunch of categories, switch it to tags, and observe the dropdown options aren't tags) but it should be working now.

@michael-genson
Copy link
Collaborator Author

Just to add: this is not a blocker for V2

@Kuchenpirat Kuchenpirat added the v2 Version 2 Issue/PR label Oct 15, 2024
@Kuchenpirat Kuchenpirat self-assigned this Oct 15, 2024
Copy link
Collaborator

@Kuchenpirat Kuchenpirat left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pretty exited for this 🚀

Did only manage to get to the frontend part today.

@michael-genson
Copy link
Collaborator Author

@Kuchenpirat let me know how you like the new translations. Unsure if they should be capitalized or not, I chose not since if they were in a sentence they wouldn't be, but I'm on the fence

@Kuchenpirat
Copy link
Collaborator

Kuchenpirat commented Oct 15, 2024

Translations are really good 👍
I think capitalization is not required here.
I also found it a bit weird the first 30 seconds looking at it, but my guess is that it is just the sharp contrast to the all caps before.

Copy link
Collaborator

@Kuchenpirat Kuchenpirat left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is not much blocking it 👍🚀🥳

Only thing is that we should add some backend validation. E.g nothing prevents me from doing this update.

PUT http://localhost:9000/api/households/cookbooks/2c435c99-e053-4ea0-a99a-94c6d37e00eb

{
  "name": "test",
  "description": "",
  "slug": "test",
  "position": 1,
  "public": false,
  "queryFilterString": "( recipe_category.id IN [\"test\"] AND tools.id IN [\"test2\"] )",
  "groupId": "0536183d-ab3b-43de-88d3-e133b37b2a36",
  "householdId": "c12d1b4f-64b6-4b05-b35e-9de739d8109e",
  "id": "2c435c99-e053-4ea0-a99a-94c6d37e00eb",
  "queryFilter": {
    "parts": [
      {
        "leftParenthesis": "(",
        "rightParenthesis": null,
        "logicalOperator": null,
        "attributeName": "recipe_category.id",
        "relationalOperator": "IN",
        "value": [
          "61a8bee6-872e-44bb-8f95-95f16c831125"
        ]
      },
      {
        "leftParenthesis": null,
        "rightParenthesis": ")",
        "logicalOperator": "AND",
        "attributeName": "tools.id",
        "relationalOperator": "IN",
        "value": [
          "6cf20798-0613-4ab9-91b2-dd0e43d8e1b1"
        ]
      }
    ]
  }
}

Only important part is: "queryFilterString": "( recipe_category.id IN [\"test\"] AND tools.id IN [\"test2\"] )",

resulting in this query_filter_string in the db:
( recipe_category.id IN ["test"] AND tools.id IN ["test2"] )

@michael-genson
Copy link
Collaborator Author

We won't be able to validate everything due to the complexity of how flexible query filter strings can be, but I can add some additional validations for sure (e.g. your example should throw an error because those are invalid UUIDs)

@Kuchenpirat
Copy link
Collaborator

Kuchenpirat commented Oct 17, 2024

Yeah my thinking was in the realm of validating ids and maybe checking if dates are parsable.
I don't think we have to overcomplicate things, as 99% of users are probably using the GUI that filters invalid entries pretty well 👍

This was just something i discovered why trying to break stuff

@michael-genson
Copy link
Collaborator Author

Should be good now! We already do validations at query execution time, so I just added those to the create/update models

Copy link
Collaborator

@Kuchenpirat Kuchenpirat left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's get that thing merged 🚀 Road to v2 🥳

@Kuchenpirat Kuchenpirat merged commit b8e62ab into mealie-recipes:mealie-next Oct 17, 2024
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
2 participants